Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: avoid zstd dependency in reth-evm, reth-evm-ethereum and reth-optimism-evm #14356

Merged
merged 3 commits into from
Feb 10, 2025
Merged

feat: avoid zstd dependency in reth-evm, reth-evm-ethereum and reth-optimism-evm #14356

merged 3 commits into from
Feb 10, 2025

Conversation

leruaa
Copy link
Contributor

@leruaa leruaa commented Feb 10, 2025

I'm currently working on updating the Reth dependency in RSP, but this time, I want RSP to depend on upstream Reth directly instead of a patched fork.

So, I need to avoid zstd being loaded in the dependency graph because it doesn't compile in SP1. And I think the easiest way to do that is to cut the reth-revm dependency in reth-evm, reth-evm-ethereum and reth-optimism-evm.

Turns out most of the types from reth-revm were re-export from revm, with the exception of the batch module, that I had to move to reth-evm.

I also had to remove reth-primitives "reth-codec" feature in reth-evm, but it don't seems to be used.

mattsse
mattsse reviewed Feb 10, 2025
View reviewed changes
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@klkvr should we move the batch file like this?

crates/ethereum/evm/Cargo.toml
@@ -15,10 +15,10 @@ workspace = true
reth-chainspec.workspace = true
reth-ethereum-forks.workspace = true
reth-evm.workspace = true
reth-primitives = { workspace = true, features = ["reth-codec"] }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this change sufficient to remove zstd from the dep graph?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately no, we still have this:

➜  cargo tree -i zstd
zstd v0.13.2
├── reth-nippy-jar v1.1.5 (/home/aurel/dev/reth/crates/storage/nippy-jar)
│   └── reth-db v1.1.5 (/home/aurel/dev/reth/crates/storage/db)
│       ├── reth-storage-api v1.1.5 (/home/aurel/dev/reth/crates/storage/storage-api)
│       │   └── reth-revm v1.1.5 (/home/aurel/dev/reth/crates/revm)
│       │       ├── reth-evm v1.1.5 (/home/aurel/dev/reth/crates/evm)
│       │       │   ├── reth-evm-ethereum v1.1.5 (/home/aurel/dev/reth/crates/ethereum/evm)
│       │       │   │   ├── rsp-client-eth v0.0.0 (/home/aurel/dev/rsp/bin/client-eth)
│       │       │   │   └── rsp-client-executor v0.1.0 (/home/aurel/dev/rsp/crates/executor/client)
│       │       │   │       └── rsp-client-eth v0.0.0 (/home/aurel/dev/rsp/bin/client-eth)
│       │       │   ├── reth-optimism-evm v1.1.5 (/home/aurel/dev/reth/crates/optimism/evm)
│       │       │   │   └── rsp-client-executor v0.1.0 (/home/aurel/dev/rsp/crates/executor/client) (*)
│       │       │   └── rsp-client-executor v0.1.0 (/home/aurel/dev/rsp/crates/executor/client) (*)
│       │       ├── reth-evm-ethereum v1.1.5 (/home/aurel/dev/reth/crates/ethereum/evm) (*)
│       │       └── reth-optimism-evm v1.1.5 (/home/aurel/dev/reth/crates/optimism/evm) (*)
│       └── reth-trie-db v1.1.5 (/home/aurel/dev/reth/crates/trie/db)
│           └── reth-storage-api v1.1.5 (/home/aurel/dev/reth/crates/storage/storage-api) (*)
└── reth-zstd-compressors v1.1.5 (/home/aurel/dev/reth/crates/storage/zstd-compressors)
    └── reth-ethereum-primitives v1.1.5 (/home/aurel/dev/reth/crates/ethereum/primitives)
        ├── reth-primitives v1.1.5 (/home/aurel/dev/reth/crates/primitives)
        │   ├── reth-db v1.1.5 (/home/aurel/dev/reth/crates/storage/db) (*)
        │   ├── reth-db-api v1.1.5 (/home/aurel/dev/reth/crates/storage/db-api)
        │   │   ├── reth-db v1.1.5 (/home/aurel/dev/reth/crates/storage/db) (*)
        │   │   ├── reth-storage-api v1.1.5 (/home/aurel/dev/reth/crates/storage/storage-api) (*)
        │   │   └── reth-trie-db v1.1.5 (/home/aurel/dev/reth/crates/trie/db) (*)
        │   ├── reth-ethereum-consensus v1.1.5 (/home/aurel/dev/reth/crates/ethereum/consensus)
        │   │   ├── reth-evm-ethereum v1.1.5 (/home/aurel/dev/reth/crates/ethereum/evm) (*)
        │   │   └── rsp-client-executor v0.1.0 (/home/aurel/dev/rsp/crates/executor/client) (*)
        │   ├── reth-evm v1.1.5 (/home/aurel/dev/reth/crates/evm) (*)
        │   ├── reth-evm-ethereum v1.1.5 (/home/aurel/dev/reth/crates/ethereum/evm) (*)
        │   ├── reth-execution-types v1.1.5 (/home/aurel/dev/reth/crates/evm/execution-types)
        │   │   ├── reth-evm v1.1.5 (/home/aurel/dev/reth/crates/evm) (*)
        │   │   ├── reth-optimism-evm v1.1.5 (/home/aurel/dev/reth/crates/optimism/evm) (*)
        │   │   ├── reth-storage-api v1.1.5 (/home/aurel/dev/reth/crates/storage/storage-api) (*)
        │   │   ├── rsp-client-executor v0.1.0 (/home/aurel/dev/rsp/crates/executor/client) (*)
        │   │   └── rsp-mpt v0.1.0 (/home/aurel/dev/rsp/crates/mpt)
        │   │       └── rsp-client-executor v0.1.0 (/home/aurel/dev/rsp/crates/executor/client) (*)
        │   ├── reth-optimism-consensus v1.1.5 (/home/aurel/dev/reth/crates/optimism/consensus)
        │   │   ├── reth-optimism-evm v1.1.5 (/home/aurel/dev/reth/crates/optimism/evm) (*)
        │   │   └── rsp-client-executor v0.1.0 (/home/aurel/dev/rsp/crates/executor/client) (*)
        │   ├── reth-optimism-evm v1.1.5 (/home/aurel/dev/reth/crates/optimism/evm) (*)
        │   ├── reth-storage-api v1.1.5 (/home/aurel/dev/reth/crates/storage/storage-api) (*)
        │   ├── rsp-client-eth v0.0.0 (/home/aurel/dev/rsp/bin/client-eth)
        │   ├── rsp-client-executor v0.1.0 (/home/aurel/dev/rsp/crates/executor/client) (*)
        │   ├── rsp-mpt v0.1.0 (/home/aurel/dev/rsp/crates/mpt) (*)
        │   ├── rsp-primitives v0.1.0 (/home/aurel/dev/rsp/crates/primitives)
        │   │   ├── rsp-client-eth v0.0.0 (/home/aurel/dev/rsp/bin/client-eth)
        │   │   ├── rsp-mpt v0.1.0 (/home/aurel/dev/rsp/crates/mpt) (*)
        │   │   └── rsp-witness-db v0.1.0 (/home/aurel/dev/rsp/crates/storage/witness-db)
        │   │       └── rsp-client-executor v0.1.0 (/home/aurel/dev/rsp/crates/executor/client) (*)
        │   └── rsp-witness-db v0.1.0 (/home/aurel/dev/rsp/crates/storage/witness-db) (*)
        ├── reth-revm v1.1.5 (/home/aurel/dev/reth/crates/revm) (*)
        └── rsp-client-executor v0.1.0 (/home/aurel/dev/rsp/crates/executor/client) (*)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah that's annyoing, the storage-api depends on trie-db which pulls in the entire db stuff...

will work on that separately

mattsse
mattsse approved these changes Feb 10, 2025
View reviewed changes
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

pending @klkvr

@mattsse mattsse mentioned this pull request Feb 10, 2025
Open
@klkvr klkvr added this pull request to the merge queue Feb 10, 2025
@mattsse mattsse added the A-dependencies Pull requests or issues that are about dependencies label Feb 10, 2025
Merged via the queue into paradigmxyz:main with commit 37adaf2 Feb 10, 2025
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattsse mattsse mattsse approved these changes

@klkvr klkvr klkvr approved these changes

@rakita rakita Awaiting requested review from rakita rakita is a code owner

@Rjected Rjected Awaiting requested review from Rjected Rjected is a code owner

@fgimenez fgimenez Awaiting requested review from fgimenez fgimenez is a code owner

@gakonst gakonst Awaiting requested review from gakonst gakonst is a code owner

Assignees
No one assigned
Labels
A-dependencies Pull requests or issues that are about dependencies
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@leruaa @mattsse @klkvr